-
Notifications
You must be signed in to change notification settings - Fork 1.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
helm: split proxy/auth documentation #19881
Conversation
63c2172
to
aec38dc
Compare
c83b7d1
to
3ec9f8f
Compare
17b4a89
to
8c4ddbf
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Took a first pass
6b68014
to
823f4b6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I reviewed purely for copy edits, and did not test anything.
docs/pages/deploy-a-cluster/helm-deployments/kubernetes-cluster.mdx
Outdated
Show resolved
Hide resolved
docs/pages/deploy-a-cluster/helm-deployments/kubernetes-cluster.mdx
Outdated
Show resolved
Hide resolved
----------------- -------- | ||
tele.example.com | ||
Login to Kubernetes by name | ||
|
||
$ tsh kube login tele.example.com | ||
|
||
# Once working, remove the KUBECONFIG= override to switch to teleport | ||
$ KUBECONFIG=${HOME?}/teleport.yaml kubectl get -n teleport-cluster pods |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
teleport.yaml
can be confused with the teleport config file. Isn't it better to rename it as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tsh kube login must also include the KUBECONFIG=${HOME?}/teleport.yaml
prefix
tsh login
no longer updates the kubeconfig - unless the --kube-cluster
flag is defined - so you can drop it from the command above.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This section always confused me as I was not aware tsh login
previously edited kubeconfig. I rewrote this bit, can you confirm it makes sense?
@ptgott @alexfornuto To follow up - is your team going to be able to give this a shot before we merge this? |
@r0mant I tested out the "kubernetes-cluster" guide and proposed edits (while working through the test plan) before I saw this PR (#20779). When are you aiming to merge this PR? Since @alexfornuto has given his approval, I'm happy to defer to him on this. |
I only ever reviewed this for copy, I did not test it. I don't think I have the prerequisite k8s knowledge to reasonably test this. |
2e5a4a9
to
6577f97
Compare
I'll merge the PR, as this is required for v12. We can do testing and finish the review a posteriori, maybe when rebasing #20779. If this is relevant to your interests, I can also provide Kubernetes training covering the parts relevant here, so we can co-own this part of the documentation. |
@hugoShaka Sounds good to me! The Docs Team is going to rework our product area ownership this quarter, so I'll let you know if any of our organizational changes apply to Kubernetes-related docs. |
@hugoShaka See the table below for backport results.
|
This PR contains documentation changes for the PR #18857.